Skip to content

refactor: add SQLite adapter to decouple from bun:sqlite#970

Merged
BYK merged 1 commit into
mainfrom
refactor/sqlite-adapter
May 20, 2026
Merged

refactor: add SQLite adapter to decouple from bun:sqlite#970
BYK merged 1 commit into
mainfrom
refactor/sqlite-adapter

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented May 15, 2026

Summary

Second step of the Bun → Node.js migration (follows #967). Introduces a SQLite adapter layer that decouples the codebase from direct bun:sqlite imports, making the database layer portable across runtimes.

Changes

  • New: src/lib/db/sqlite.ts — SQLite adapter using node:sqlite (Node 22+)
    • Falls back to bun:sqlite when node:sqlite is unavailable (during Bun test runner transition only — fallback will be removed once tests migrate to Vitest)
    • Handles API differences: prefers bun:sqlite's cached .query() when available, falls back to node:sqlite's .prepare(); provides manual BEGIN/COMMIT/ROLLBACK wrapper for node:sqlite (which lacks native .transaction())
    • Normalizes get() return value to null (not undefined) for no-row results, matching codebase convention
    • Exports Database class and SQLQueryBindings type
  • Fixed: isSchemaError() and isReadonlyError() in schema.ts — now match by error message content instead of error.name === "SQLiteError", making auto-repair and readonly detection work under node:sqlite (which throws plain Error, not SQLiteError)
  • Updated imports in 4 source files: db/index.ts, schema.ts, migration.ts, utils.ts
  • Updated imports in 3 test files: fix.test.ts, telemetry.test.ts, schema.test.ts
  • Updated: lint-rules/no-manual-transactions.grit — excludes db/sqlite.ts (manual transactions are necessary in the adapter for node:sqlite compatibility)
  • Added: MIGRATION-PLAN.md documenting remaining migration steps

Design

Call sites continue using .query(sql).get() / .all() / .run() and db.transaction() exactly as before — no migration churn needed.

Zero bun:sqlite imports remain in src/ or test/.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-970/

Built to branch gh-pages at 2026-05-20 13:51 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

Codecov Results 📊

7012 passed | Total: 7012 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

✅ Patch coverage is 85.25%. Project has 14087 uncovered lines.
✅ Project coverage is 77.24%. Comparing base (base) to head (head).

Files with missing lines (2)
File Patch % Lines
src/lib/db/sqlite.ts 82.98% ⚠️ 8 Missing
src/lib/db/schema.ts 92.31% ⚠️ 1 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    77.12%    77.24%    +0.12%
==========================================
  Files          320       321        +1
  Lines        61947     61889       -58
  Branches         0         0         —
==========================================
+ Hits         47773     47802       +29
- Misses       14174     14087       -87
- Partials         0         0         —

Generated by Codecov Action

@BYK BYK force-pushed the refactor/sqlite-adapter branch from c0b3adf to 972b864 Compare May 15, 2026 17:54
Comment thread src/lib/db/sqlite.ts
Comment thread src/lib/db/sqlite.ts
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 972b864. Configure here.

Comment thread src/lib/db/sqlite.ts Outdated
@BYK BYK force-pushed the refactor/sqlite-adapter branch 3 times, most recently from 139bea9 to 4e51cb0 Compare May 20, 2026 13:42
Introduce src/lib/db/sqlite.ts — a runtime-detecting adapter that
provides a unified Database API for SQLite access. Under Bun it
re-exports bun:sqlite directly (zero overhead). Under Node.js it
wraps node:sqlite's DatabaseSync with the same .query()/.exec()/
.close()/.transaction() interface.

This eliminates all direct bun:sqlite imports from src/ and test/,
making the DB layer portable across runtimes without changing any
call sites.

Changes:
- New: src/lib/db/sqlite.ts (adapter with runtime detection)
- Updated: db/index.ts, schema.ts, migration.ts, utils.ts imports
- Updated: 3 test files to import from adapter
- Added: MIGRATION-PLAN.md documenting remaining migration steps
@BYK BYK force-pushed the refactor/sqlite-adapter branch from 4e51cb0 to c023df4 Compare May 20, 2026 13:51
@BYK BYK merged commit d6d69e3 into main May 20, 2026
30 checks passed
@BYK BYK deleted the refactor/sqlite-adapter branch May 20, 2026 13:58
BYK added a commit that referenced this pull request May 20, 2026
## Summary

Third step of the Bun → Node.js migration (follows #967, #970). Replaces
all Bun-specific API calls in `src/` with Node.js equivalents.

### Changes

**File I/O** (18 files):
- `Bun.file(path).text()` → `readFile(path, "utf-8")` from
`node:fs/promises`
- `Bun.file(path).json()` → `JSON.parse(await readFile(path, "utf-8"))`
- `Bun.file(path).exists()` → `access(path).then(() => true, () =>
false)`
- `Bun.file(path).stat()` → `stat(path)` from `node:fs/promises`
- `Bun.write(path, content)` → `writeFile(path, content)` from
`node:fs/promises`
- `Bun.write(dest, Bun.file(src))` → `copyFile(src, dest)` from
`node:fs/promises`

**Process/System** (9 files + 1 new):
- `Bun.which()` → new `src/lib/which.ts` helper using `command -v`
(POSIX) / `where` (Windows)
- `Bun.spawn()` → `spawn()` from `node:child_process` with
Promise-wrapped exit code
- `Bun.spawnSync()` → `spawnSync()` from `node:child_process`
- `Bun.sleep()` → `setTimeout()` from `node:timers/promises`

**Utilities** (6 files):
- `Bun.Glob` → `picomatch` (already a devDependency)
- `Bun.randomUUIDv7()` → `uuidv7()` from `uuidv7` package
- `Bun.semver.order()` → `compare()` from `semver` package

### What's NOT in this PR (Group D — separate follow-up)

These Bun APIs in `bspatch.ts` and `upgrade.ts` require more careful
handling:
- `Bun.file().writer()` — streaming file writer (needs
`fs.createWriteStream`)
- `Bun.zstdCompress/DecompressSync` — zstd compression (needs
`node:zlib` 22.15+)
- `Bun.mmap()` — memory-mapped files (has existing fallback)
- `Bun.CryptoHasher` — streaming hash (needs `crypto.createHash`)

### Test results

All 7012 unit tests pass, 0 failures.
BYK added a commit that referenced this pull request May 20, 2026
…writer) (#986)

## Summary

Fourth step of the Bun → Node.js migration (follows #967, #970, #984).
Replaces the remaining Bun-specific APIs in `src/` — the "Group D" items
that required more careful handling.

### Changes

**`src/lib/bspatch.ts`** (rewritten):
- `Bun.zstdDecompressSync()` → `zstdDecompressSync()` from `node:zlib`
- `DecompressionStream('zstd')` → `createZstdDecompress()` from
`node:zlib` piped through a Node Readable→Web ReadableStream bridge
- `Bun.mmap()` → removed entirely; uses `readFile()` for both paths
(copy-then-read and direct-read fallback)
- `Bun.CryptoHasher("sha256")` → `createHash("sha256")` from
`node:crypto`
- `Bun.file(destPath).writer()` → `createWriteStream(destPath)` from
`node:fs`

**`src/lib/upgrade.ts`**:
- `Bun.file(destPath).writer()` → `createWriteStream(destPath)` from
`node:fs`

**`src/lib/api/sourcemaps.ts`**:
- `Bun.zstdCompress(buf, { level: 3 })` → `zstdCompress()` from
`node:zlib`

**`src/lib/telemetry/zstd-transport.ts`**:
- Removed `globalThis.Bun.zstdCompress` dynamic access and `BunZstdHost`
type
- Replaced with direct `zstdCompress` from `node:zlib` (always available
in Node 22.15+)
- Removed belt-and-braces fallback (zstd can't disappear at runtime with
`node:zlib`)
- `hasZstdSupport()` now checks `typeof zstdCompressCb === "function"`
directly

**Tests updated** (`test/lib/telemetry/zstd-transport.test.ts`):
- Removed 4 tests for `globalThis.Bun.zstdCompress` fallback paths (no
longer applicable)
- Replaced `Bun.zstdDecompress` with `zstdDecompressSync` in assertions

### Result

**Zero non-comment `Bun.*` API calls remain in `src/`.** The only `bun:`
reference left is the `require("bun:sqlite")` fallback in `sqlite.ts`,
which will be removed when the test runner migrates to Vitest.

All 7014 tests pass, 0 failures.
BYK added a commit that referenced this pull request May 20, 2026
Consolidates fixes for review comments across PRs #967, #970, #984, #986.

Changes:
- Bump engines.node from >=22.12 to >=22.15 (zstd APIs require 22.15+,
  Node 20 is EOL). Update dist/bin.cjs runtime version check to match.
- Simplify zstd imports: replace feature-detection dance with direct
  imports from node:zlib now that >=22.15 is guaranteed.
- Fix spawn error handling in upgrade.ts: propagate error object to catch
  block so ENOENT/EBUSY detection works (was silently resolving with 1).
- Fix sqlite.ts transaction(): wrap ROLLBACK in try/catch so original
  error is preserved if ROLLBACK itself fails.
- Guard semver.compare calls in delta-upgrade.ts with semver.valid() to
  avoid throwing on invalid version strings.
- Narrow catch in shell.ts addToGitHubPath to ENOENT only, re-throw
  other errors (EACCES, EPERM) instead of silently swallowing.
- Add WriteStream backpressure handling in upgrade.ts
  streamDecompressToFile: check write() return value, await drain.
- Add setup-node@v6 with Node 22 to E2E CI job.
- Fix whichSync Windows CRLF: split on /\r?\n/ instead of \n.

7014 tests pass, 0 failures.
BYK added a commit that referenced this pull request May 20, 2026
Consolidates fixes for review comments across PRs #967, #970, #984, #986.

Changes:
- Bump engines.node from >=22.12 to >=22.15 (zstd APIs require 22.15+,
  Node 20 is EOL). Update dist/bin.cjs runtime version check to match.
- Simplify zstd imports: replace feature-detection dance with direct
  imports from node:zlib now that >=22.15 is guaranteed.
- Fix spawn error handling in upgrade.ts: propagate error object to catch
  block so ENOENT/EBUSY detection works (was silently resolving with 1).
- Fix sqlite.ts transaction(): wrap ROLLBACK in try/catch so original
  error is preserved if ROLLBACK itself fails.
- Guard semver.compare calls in delta-upgrade.ts with semver.valid() to
  avoid throwing on invalid version strings.
- Narrow catch in shell.ts addToGitHubPath to ENOENT only, re-throw
  other errors (EACCES, EPERM) instead of silently swallowing.
- Add WriteStream backpressure handling in upgrade.ts
  streamDecompressToFile: check write() return value, await drain.
- Add setup-node@v6 with Node 22 to E2E CI job.
- Fix whichSync Windows CRLF: split on /\r?\n/ instead of \n.

7014 tests pass, 0 failures.
BYK added a commit that referenced this pull request May 20, 2026
Consolidates fixes for review comments across PRs #967, #970, #984, #986.

Changes:
- Bump engines.node from >=22.12 to >=22.15 (zstd APIs require 22.15+,
  Node 20 is EOL). Update dist/bin.cjs runtime version check to match.
- Simplify zstd imports: replace feature-detection dance with direct
  imports from node:zlib now that >=22.15 is guaranteed.
- Fix spawn error handling in upgrade.ts: propagate error object to catch
  block so ENOENT/EBUSY detection works (was silently resolving with 1).
- Fix sqlite.ts transaction(): wrap ROLLBACK in try/catch so original
  error is preserved if ROLLBACK itself fails.
- Guard semver.compare calls in delta-upgrade.ts with semver.valid() to
  avoid throwing on invalid version strings.
- Narrow catch in shell.ts addToGitHubPath to ENOENT only, re-throw
  other errors (EACCES, EPERM) instead of silently swallowing.
- Add WriteStream backpressure handling in upgrade.ts
  streamDecompressToFile: check write() return value, await drain.
- Add setup-node@v6 with Node 22 to E2E CI job.
- Fix whichSync Windows CRLF: split on /\r?\n/ instead of \n.

7014 tests pass, 0 failures.
BYK added a commit that referenced this pull request May 20, 2026
## Summary

Consolidates fixes for review comments across PRs #967, #970, #984,
#986, superseding #988.

### Critical
- **Bump `engines.node` to `>=22.15`** — `node:zlib` zstd APIs require
22.15+. Node 20 is EOL. Updated `dist/bin.cjs` runtime version check to
match.
- **Simplify zstd imports** — replaced feature-detection dance with
direct `import { zstdCompress } from "node:zlib"` now that >=22.15 is
guaranteed.

### High
- **Fix spawn error handling in `upgrade.ts`** — `proc.on("error", () =>
resolve(1))` discarded the error object, making ENOENT/EBUSY detection
dead code. Now properly rejects with the error.

### Medium
- **Fix `sqlite.ts` ROLLBACK** — if ROLLBACK throws, the original
transaction error was lost. Now wrapped in try/catch.
- **Guard `semver.compare`** in `delta-upgrade.ts` with `semver.valid()`
to avoid throwing on invalid version strings.
- **Narrow catch in `shell.ts`** `addToGitHubPath` — only catch ENOENT,
re-throw EACCES/EPERM.
- **Add WriteStream backpressure** in `upgrade.ts`
`streamDecompressToFile` — check `write()` return value, await
`'drain'`.
- **Add `setup-node@v6`** with Node 22 to E2E CI job.

### Low
- **Fix `whichSync` Windows CRLF** — split on `/\r?\n/` instead of `\n`
to strip trailing `\r` from `where.exe` output.

### Test results
All 7014 tests pass, 0 failures.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant